fix: describe silently dropped trailing XPath constraint groups (#772) - #80
Merged
Conversation
…ixlabs#772) Mendix stores sibling predicate groups concatenated in one XPathConstraint field: [Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]] [Status != 'Completed'] [CompletionDate = empty] The grammar's xpathConstraint rule matches ONE bracket group. ParseXPathConstraint removes the error listeners, so ANTLR parsed the first group, left the rest on the token stream, and still returned ok=true. enrichXPathConstraintForDescribe read that as a full parse and re-rendered only what came back — its `if !ok { return original }` fallback never fired — so describe emitted: where Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]; That is worse than a crash. The output looks complete while describing a materially less restrictive query than the project contains, which makes correct defensive code read as buggy — and `describe` is what an agent reads to decide whether code is right. Fixed in two layers: 1. ParseXPathConstraint reports a partial parse as a failure (require the token stream to be at EOF). That alone stops the data loss: the caller falls back to the stored string, which the render path then splits correctly. 2. visitor.SplitXPathPredicateGroups splits a constraint into its top-level groups, and each is enriched and rendered separately — so enum enrichment reaches groups after the first, not just the first. The splitter tracks nesting depth and quoting, because the previous "][" split mangled both a nested [A/B[x = 1]] and a literal containing ']'. The render path now uses it too. Verified end-to-end on a real 11.12.2 project carrying the reported constraint shape: all three groups render, Status is enriched to its qualified enum value in the second group, the output re-parses and re-executes to an identical flow, and `mx check` reports 0 errors. A/B against a pre-fix binary on the same project reproduces the two dropped groups exactly as reported. All three guards mutation-checked. Refs mendixlabs#772
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the upstream report mendixlabs/mxcli#772.
Problem
Mendix stores sibling predicate groups concatenated in a single
XPathConstraintfield:describerendered only the first:The reporter's framing is the important part: this is worse than a crash. The output looks complete while describing a materially less restrictive query than the project contains — so correct defensive code reads as buggy, and
describeis exactly what an agent reads to decide whether code is right.Root cause
The grammar's
xpathConstraintrule matches one bracket group.ParseXPathConstraintremoves the error listeners, so ANTLR parsed the first group, left the rest on the token stream, and still returnedok = true.enrichXPathConstraintForDescribetreated that as a full parse and re-rendered only what came back — itsif !ok { return original }fallback never fired.Confirmed directly before touching anything:
Approach — two layers
ParseXPathConstraintrejects a partial parse (require the token stream at EOF). This alone stops the data loss: the caller falls back to the stored string, which the render path then splits correctly.visitor.SplitXPathPredicateGroupssplits a constraint into top-level groups, each enriched and rendered separately — so enum enrichment reaches groups after the first, not just the first.The splitter tracks nesting depth and quoting, because the previous
"]["split mangles both a nested[A/B[x = 1]]and a literal containing]([Name = 'a]b']). Both appear in real projects, and mishandling either silently changes what a query means. The render path now uses the same splitter instead of its own ad-hoc version.A group that does not parse is passed through verbatim rather than dropped or guessed at.
Verification
End-to-end on a real 11.12.2 project carrying the reported constraint shape:
where Bug772.Task_TaskGroup/Bug772.TaskGroup[EndDate = $EndDateLimit];Status != Bug772.TaskStatus.Completedenriched in the second groupmxcli docker check→ 0 errors../...suite green.All three guards mutation-checked. Worth noting what that revealed: reverting the per-group split alone does not reproduce the data loss any more, because the strict parse already prevents it — it only degrades enum enrichment (
'Completed'instead of the qualified value). That is defence in depth working as intended, and it is why there is a separateenrichXPathGroupsunit test rather than relying on the end-to-end one.Generalisable lesson (recorded in the symptom table)
A parser that silently accepts a prefix is worse than one that fails. Any
okreturned by a rule that can match less than its input must be checked against EOF before a caller treats the result as lossless.Repro script:
mdl-examples/bug-tests/772-xpath-constraint-groups.mdl.Generated by Claude Code